Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: call conn.release method in ext.ReleaseBodystream #1252

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Duslia
Copy link
Member

@Duslia Duslia commented Dec 24, 2024

What type of PR is this?

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

在 ext.ReleaseBodystream 中调用 conn.Release() 方法

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en: In a streaming scenario, if there is a large amount of remaining data on the connection, all of it will be read into memory when skipRest is performed, which may lead to an OOM error.
zh(optional): 在流式场景下,如果连接上剩余数据很多时,在 skipRest 时会全部读到内存中,可能导致 OOM。

(Optional) Which issue(s) this PR fixes:

(Optional) The PR that updates user documentation:

@Duslia Duslia requested review from a team as code owners December 24, 2024 07:08
@Duslia Duslia force-pushed the fix/release_body_stream branch 2 times, most recently from b3eba17 to bbedaf7 Compare December 24, 2024 07:56
@@ -1425,6 +1425,7 @@ func TestStreamNotEnoughData(t *testing.T) {
err = ext.ReleaseBodyStream(req.BodyStream())
assert.Nil(t, err)
assert.DeepEqual(t, 0, len(conn.Data))
assert.DeepEqual(t, true, conn.HasReleased)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加单测模拟这个下这个问题,这个单测不够

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个内存高咋模拟呀

@Duslia Duslia force-pushed the fix/release_body_stream branch from bbedaf7 to b5534a4 Compare December 25, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants